Skip to content

fix(db-ivm): treat 0 and empty string as valid min/max extremes - #1809

Open
aakbarpour wants to merge 1 commit into
TanStack:mainfrom
aakbarpour:cursor/db-minmax-falsy-a997
Open

fix(db-ivm): treat 0 and empty string as valid min/max extremes#1809
aakbarpour wants to merge 1 commit into
TanStack:mainfrom
aakbarpour:cursor/db-minmax-falsy-a997

Conversation

@aakbarpour

@aakbarpour aakbarpour commented Sep 11, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

🎯 Changes

min() and max() in @tanstack/db-ivm treated a missing accumulator as any falsy value. A group whose true extreme is 0, 0n, or "" therefore returned the wrong aggregate.

The reducers now compare against undefined only.

Fixes #1775

✅ Checklist

  • I have tested this code locally with pnpm test.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Bug Fixes

    • Fixed min and max grouping results so valid falsy values—including 0, 0n, and empty strings—are correctly considered.
    • Grouped calculations now return accurate minimum and maximum values when data includes zero or other falsy values.

  • Tests

    • Added coverage for falsy extremes and grouped results containing zero values.

@aakbarpour
fix(db-ivm): treat 0 and empty string as valid min/max extremes
a08f545 
min() and max() used truthiness to detect an unset accumulator, so 0,
0n, and "" were skipped or overwritten. Compare against undefined.

Fixes TanStack#1775
@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 45772ad8-fa48-46ab-bbb2-befde4284dcf

📥 Commits

Reviewing files that changed from the base of the PR and between 743d11a and a08f545.

📒 Files selected for processing (3)
  • .changeset/minmax-falsy-extremes.md
  • packages/db-ivm/src/operators/groupBy.ts
  • packages/db-ivm/tests/operators/groupBy.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

The min and max reducers now use undefined as the unset sentinel. New tests cover 0, 0n, empty strings, and grouped results containing zero. A patch changeset documents the correction.

Changes

Falsy min/max aggregate handling

Layer / File(s) Summary
Update min/max reducer sentinel checks
packages/db-ivm/src/operators/groupBy.ts		 The min and max reducers now compare against undefined and accept falsy values as valid candidates.
Add regression tests and release metadata
packages/db-ivm/tests/operators/groupBy.test.ts, .changeset/minmax-falsy-extremes.md		 Tests cover falsy extremes and grouped results containing zero. The changeset declares a patch release.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: kyleamathews

Merge Risk: ⚪ Minimal · up to a08f5

Falsy numeric, bigint, and string extremes are preserved with regression coverage for direct and grouped aggregates. The change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: treating 0 and empty strings as valid min/max extremes in db-ivm.
Description check ✅ Passed The description includes the required Changes, Checklist, and Release Impact sections. It explains the bug and fix, reports local testing, identifies the published-code impact, and includes a changese…
Linked Issues check ✅ Passed The change satisfies #1775. packages/db-ivm/src/operators/groupBy.ts now uses minValue === undefined and maxValue === undefined as the accumulator checks. The reducers compare values with < an…
Out of Scope Changes check ✅ Passed The changes stay within #1775. The source update fixes the two affected reducers. The added tests verify the regression and grouped behavior. The changeset documents the patch release for `@tanstack/d…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

db-ivm: min()/max() aggregates return wrong results when the extreme value is falsy (0 or empty string)

1 participant

@aakbarpour